Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Build fixes for GCC 13 and MinGW compilers #52

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Build fixes for GCC 13 and MinGW compilers #52

merged 2 commits into from
Apr 22, 2024
+36 −32

Conversation

hhromic
Copy link
Contributor

@hhromic hhromic commented Apr 21, 2024 •
edited
Loading

This PR fixes two build issues I found while preparing new updated Solarus build images that support Qlementine.

  1. Fix dangling reference warnings in GCC 13
  2. Use correct preprocessor macro for Windows code paths

Each of these is fixed on an individual commit for an easier review, but explanations are also provided below.

In GCC 13, a new dangling references warning was introduced. Qlementine has code that is triggering this warning and failing to build due to -Werror. This PR fixes the warnings-turned-errors by using intermediate assignments instead.

The first commit in this PR fixes these dangling reference warnings. 
/qlementine/lib/src/widgets/Switch.cpp: In member function ‘const QColor& oclero::qlementine::Switch::getBgColor() const’:
/qlementine/lib/src/widgets/Switch.cpp:251:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  251 |   const auto& bgColor = qlementineStyle ? qlementineStyle->switchGrooveColor(
      |               ^~~~~~~
/qlementine/lib/src/widgets/Switch.cpp:253:73: note: the temporary was destroyed at the end of the full expression ‘QStyle::standardPalette().QPalette::color((((const oclero::qlementine::Switch*)this)->oclero::qlementine::Switch::<anonymous>.QAbstractButton::<anonymous>.QWidget::isEnabled() ? QPalette::Normal :  QPalette::Disabled), QPalette::Button)’
  253 |                                         : style->standardPalette().color(
      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
  254 |                                           isEnabled() ? QPalette::ColorGroup::Normal : QPalette::ColorGroup::Disabled,
      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  255 |                                           QPalette::ColorRole::Button);
      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/qlementine/lib/src/widgets/Switch.cpp: In member function ‘const QColor& oclero::qlementine::Switch::getBorderColor() const’:
/qlementine/lib/src/widgets/Switch.cpp:262:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  262 |   const auto& borderColor =
      |               ^~~~~~~~~~~
/qlementine/lib/src/widgets/Switch.cpp:266:39: note: the temporary was destroyed at the end of the full expression ‘QStyle::standardPalette().QPalette::color((((const oclero::qlementine::Switch*)this)->oclero::qlementine::Switch::<anonymous>.QAbstractButton::<anonymous>.QWidget::isEnabled() ? QPalette::Normal :  QPalette::Disabled), QPalette::ButtonText)’
  266 |       : style->standardPalette().color(
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
  267 |         isEnabled() ? QPalette::ColorGroup::Normal : QPalette::ColorGroup::Disabled, QPalette::ColorRole::ButtonText);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/qlementine/lib/src/widgets/Switch.cpp: In member function ‘const QColor& oclero::qlementine::Switch::getFgColor() const’:
/qlementine/lib/src/widgets/Switch.cpp:274:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  274 |   const auto& fgColor = qlementineStyle ? qlementineStyle->switchHandleColor(
      |               ^~~~~~~
/qlementine/lib/src/widgets/Switch.cpp:276:73: note: the temporary was destroyed at the end of the full expression ‘QStyle::standardPalette().QPalette::color((((const oclero::qlementine::Switch*)this)->oclero::qlementine::Switch::<anonymous>.QAbstractButton::<anonymous>.QWidget::isEnabled() ? QPalette::Normal :  QPalette::Disabled), QPalette::ButtonText)’
  276 |                                         : style->standardPalette().color(
      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
  277 |                                           isEnabled() ? QPalette::ColorGroup::Normal : QPalette::ColorGroup::Disabled,
      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  278 |                                           QPalette::ColorRole::ButtonText);
      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/qlementine/lib/src/widgets/Switch.cpp: In member function ‘const QColor& oclero::qlementine::Switch::getTextColor() const’:
/qlementine/lib/src/widgets/Switch.cpp:285:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  285 |   const auto& textColor =
      |               ^~~~~~~~~
/qlementine/lib/src/widgets/Switch.cpp:288:39: note: the temporary was destroyed at the end of the full expression ‘QStyle::standardPalette().QPalette::color((((const oclero::qlementine::Switch*)this)->oclero::qlementine::Switch::<anonymous>.QAbstractButton::<anonymous>.QWidget::isEnabled() ? QPalette::Normal :  QPalette::Disabled), QPalette::Text)’
  288 |       : style->standardPalette().color(
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
  289 |         isEnabled() ? QPalette::ColorGroup::Normal : QPalette::ColorGroup::Disabled, QPalette::ColorRole::Text);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The second build error is due to (incorrectly) using a WIN32 user macro instead of the standard predefined _WIN32 compiler macro. When compiling with MSVC, it automatically adds the user defined macro WIN32 for backwards compatibility, but this is not the case for other compilers such as MinGW or Clang. Therefore, when compiling with these, the wrong code paths are chosen causing a missing symbol in the built library that then causes a linking error in client applications.

The second commit replaces `WIN32` with `_WIN32` and fixes this linking error in client applications. 
[ 98%] Linking CXX executable sandbox.exe
/usr/lib/gcc/x86_64-w64-mingw32/13.1.0/../../../../x86_64-w64-mingw32/bin/ld: ../lib/libqlementine.a(ResourceInitialization.cpp.obj):ResourceInitialization.cpp:(.text+0x18): undefined reference to `qInitResources_qlementine_font_inter()'
/usr/lib/gcc/x86_64-w64-mingw32/13.1.0/../../../../x86_64-w64-mingw32/bin/ld: ../lib/libqlementine.a(ResourceInitialization.cpp.obj):ResourceInitialization.cpp:(.text+0x38): undefined reference to `qInitResources_qlementine_font_inter()'
collect2: error: ld returned 1 exit status
make[2]: *** [sandbox/CMakeFiles/sandbox.dir/build.make:185: sandbox/sandbox.exe] Error 1
make[1]: *** [CMakeFiles/Makefile2:203: sandbox/CMakeFiles/sandbox.dir/all] Error 2
make: *** [Makefile:91: all] Error 2

EDIT: I tried to run clang-format but it formatted more code than my own changes in this PR, therefore I think is better to run it outside of this particular PR to reduce diff noise.

hhromic added 2 commits April 21, 2024 21:43
@hhromic
Fix dangling reference warnings in GCC 13
99c3134 
* The code is essentially the same but the compiler is happier.
@hhromic
Use correct preprocessor macro for Windows code paths
1da9401 
* The `WIN32` _macro_ is not defined by any compiler but only some build systems.
  - Not to be confused with the `WIN32` _variable_ in CMake.
* The standardized macro to use is `_WIN32` which is defined by most Windows compilers.
  - Reference: <https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros>
* This fixes Qlementine when using MinGW and other non-MSVC compilers.
@hhromic hhromic changed the title Build fixes for GCC 13 and MinGW compilers. Build fixes for GCC 13 and MinGW compilers Apr 21, 2024
@oclero oclero added this to the 1.0.0 milestone Apr 22, 2024
@oclero oclero assigned oclero and unassigned oclero Apr 22, 2024
@oclero oclero self-requested a review April 22, 2024 18:07
oclero
oclero approved these changes Apr 22, 2024
View reviewed changes
Copy link
Owner

@oclero oclero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🙏

@oclero oclero merged commit 487d3f7 into oclero:master Apr 22, 2024
3 checks passed
@hhromic hhromic deleted the build-fixes branch April 22, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oclero oclero oclero approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@hhromic @oclero